Skip to content

Adds the ed25519 functions to the python binding. - #8257

Open
jameskeane wants to merge 1 commit into
arvidn:masterfrom
jameskeane:add-ed25519-python-binding
Open

Adds the ed25519 functions to the python binding.#8257
jameskeane wants to merge 1 commit into
arvidn:masterfrom
jameskeane:add-ed25519-python-binding

Conversation

@jameskeane

Copy link
Copy Markdown

ed25519 private keys in the 64 byte representation are hard to make as most cryptographic libraries in python use the 32 byte seed.

Exposing these functions will make it easier to integrate libtorrent's DHT functions in python.

ed25519 private keys in the 64 byte representation are hard to make
as most cryptographic libraries in python use the 32 byte seed.

Exposing these functions will make it easier to integrate libtorrent's
DHT functions in python.
@arvidn
arvidn requested a review from Copilot April 7, 2026 12:06

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds Ed25519 (DHT) helper functions to the Python bindings to support 32-byte seed → 64-byte secret key workflows and related operations.

Changes:

  • Introduces Boost.Python bindings for seed generation, keypair creation, sign/verify, scalar addition, and key exchange.
  • Registers the new binding module entry point in module.cpp.
  • Adds build system entries (CMake + Jamfile) and a Python test suite covering the new APIs.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
bindings/python/tests/ed25519_test.py Adds unit tests for the new ed25519 Python APIs (sizes, determinism, sign/verify, scalar ops, key exchange).
bindings/python/src/module.cpp Wires bind_ed25519() into the Python module initialization.
bindings/python/src/ed25519.cpp Implements the Boost.Python bindings that wrap libtorrent’s dht::ed25519_* functions.
bindings/python/Jamfile Ensures src/ed25519.cpp is built into the Boost.Build Python extension.
bindings/python/CMakeLists.txt Ensures src/ed25519.cpp is built into the CMake Python module target.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

// file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt)

#include "boost_python.hpp"
#include <libtorrent/kademlia/ed25519.hpp>

Copilot AI Apr 7, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file uses std::array, std::copy, std::invalid_argument, and std::ptrdiff_t but doesn’t include the corresponding standard headers. Relying on transitive includes can break builds depending on include order/toolchain. Add explicit includes for the standard library facilities used (e.g., , , , and ).

Suggested change
#include <libtorrent/kademlia/ed25519.hpp>
#include <libtorrent/kademlia/ed25519.hpp>
#include <algorithm>
#include <array>
#include <cstddef>
#include <stdexcept>

Copilot uses AI. Check for mistakes.
Comment on lines +11 to +14
def test_seeds_are_unique(self) -> None:
seed1 = lt.ed25519_create_seed()
seed2 = lt.ed25519_create_seed()
self.assertNotEqual(seed1, seed2)

Copilot AI Apr 7, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test is probabilistic: two consecutive random seeds can (in theory) be equal, which can introduce an extremely rare but real CI flake. Consider removing the uniqueness assertion, or replacing it with a non-probabilistic property test (e.g., only assert length/type here, and keep determinism tests focused on create_keypair with a fixed input).

Copilot uses AI. Check for mistakes.
self.assertNotEqual(pk1, pk2)

def test_invalid_seed_length(self) -> None:
with self.assertRaises(Exception):

Copilot AI Apr 7, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

assertRaises(Exception) is very broad and can mask unexpected failures (e.g., crashes in the binding vs. a deliberate argument validation error). Prefer asserting a more specific exception type that the bindings are expected to raise for invalid input (once standardized), so tests validate API behavior rather than just “something failed.”

Suggested change
with self.assertRaises(Exception):
with self.assertRaises(ValueError):

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants